Skip to content

Conversation

@tomi-font
Copy link
Contributor

@tomi-font tomi-font commented Feb 13, 2025

@nordic-piks
Copy link
Contributor

@tomi-font Any update here? We see failures in CI with those tests.

@tomi-font
Copy link
Contributor Author

@tomi-font Any update here? We see failures in CI with those tests.

This is still a WIP. Part of the issues have been fixed.

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch 3 times, most recently from 8854ce6 to 04dac73 Compare March 3, 2025 15:36
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 04dac73 to 804577c Compare March 4, 2025 09:23
@tomi-font
Copy link
Contributor Author

@nordic-piks FYI everything is now fixed with nrfconnect/sdk-nrf#20710.

@Vge0rge
Copy link
Contributor

Vge0rge commented Mar 13, 2025

Code looks ok, you just need to update the commit message for the dynamic slots as you did in the upstream pr.

@alxelax
Copy link
Contributor

alxelax commented Mar 13, 2025

@tomi-font, will you add Zephyr's key ID distribution approach?

@tomi-font
Copy link
Contributor Author

Code looks ok, you just need to update the commit message for the dynamic slots as you did in the upstream pr.

Yeah, I'm planning to do the cherry picking again when the upstream PR is merged to have the commits be fromtrees.

@tomi-font, will you add Zephyr's key ID distribution approach?

Yes. I'll do all the changes at once.

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 804577c to 0be8c38 Compare March 17, 2025 07:59
@tomi-font
Copy link
Contributor Author

Converted the fromlists to fromtrees and rebased.

@tomi-font tomi-font requested review from Vge0rge and alxelax March 17, 2025 08:00
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 0be8c38 to e833532 Compare March 17, 2025 14:13
Copy link
Contributor

@frkv frkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tomi-font and others added 9 commits March 20, 2025 13:03
This reverts commit 8a64a2e.

We shouldn't have noups to fix things that can
and should be fixed elsewhere/differently.

Signed-off-by: Tomi Fontanilles <[email protected]>
This reverts commit fcb4238.

We shouldn't have noups to fix things that can
and should be fixed elsewhere/differently.

Signed-off-by: Tomi Fontanilles <[email protected]>
…ofile

TF-M small profile does not support secure storage (know as Protected
storage), this commit add filter for tfm test case to pass it
incase of small profile been set, see tf-m profiles in below link

https://tf-m-user-guide.trustedfirmware.org/configuration/profiles/index.html

Signed-off-by: Sadik Ozer <[email protected]>
(cherry picked from commit 6932885)
Signed-off-by: Tomi Fontanilles <[email protected]>
…nabled scenarios

Explicitly set the TF-M profile to not rely on the build system defaults
which might differ.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit 62fe34d)
Signed-off-by: Tomi Fontanilles <[email protected]>
The psa_key_attributes_t type is implementation-defined according to
the PSA Crypto spec.
Compare its fields individually instead of doing a memcmp() over the
entire struct.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit 744e9f7)
Signed-off-by: Tomi Fontanilles <[email protected]>
…ples/tests

Explicitly enable CONFIG_ENTROPY_GENERATOR instead of relying on the
build system's defaults.

This:
- Makes sure the filtering works properly between entropy_driver and
entropy_not_secure test scenarios for the samples.
- Helps with TF-M builds in certain scenarios where key generation (via
`psa_generate_key()`) would fail due to the RNG functionality being
disabled.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit 25ad578)
Signed-off-by: Tomi Fontanilles <[email protected]>
Use dynamic allocation for key material for
better compatibility as a fully static key store is a new
feature that not all PSA Crypto implementations support.

Explicitly enable CONFIG_MBEDTLS_ENABLE_HEAP to ensure that Mbed TLS uses
heap for the PSA keys' data (instead of failing at runtime).
This will turn off CONFIG_MBEDTLS_PSA_STATIC_KEY_SLOTS, making the
implementation default to dynamic key slots.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit eb1ed12)
Signed-off-by: Tomi Fontanilles <[email protected]>
…YPTO_STORAGE_C

Add a Kconfig option to match the Mbed TLS define
instead of defining it based on CONFIG_SECURE_STORAGE.

This gives more flexibility regarding the potential re-definition of the
CONFIG_MBEDTLS_PSA_CRYPTO_STORAGE_C Kconfig option.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit 8627250)
Signed-off-by: Tomi Fontanilles <[email protected]>
Some implementations require more stack than others.
Increase the Ztest and main stack sizes to accommodate them.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit f50c3d9)
Signed-off-by: Tomi Fontanilles <[email protected]>
On top of enabling and allowing test entropy sources, enable
CONFIG_ENTROPY_GENERATOR so that a real driver and entropy source gets
used if available.
This is needed for some PSA Crypto implementations that have random number
generation conditionally compiled in.

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit b920686)
Signed-off-by: Tomi Fontanilles <[email protected]>
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from e833532 to d6fe2d1 Compare March 20, 2025 11:03
@sonarqubecloud
Copy link

Copy link
Contributor

@frkv frkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

Copy link
Contributor

@Vge0rge Vge0rge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and these are clean cherry picks.

@nordicjm nordicjm merged commit ee5c856 into nrfconnect:main Mar 24, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants